fix: interactive-prd workflow bugs (#1001, #1002, #1003)#1005
Merged
Conversation
…euse conversations (#1001, #1002, #1003) The archon-interactive-prd workflow was completely non-functional due to three bugs: approval gates didn't capture user responses (missing capture_response: true), the generate node wrote to .claude/ which the Claude SDK blocks, and CLI approve/reject created new conversations instead of reusing the original. - Add capture_response: true to all three approval gates in the YAML - Change output path from .claude/PRPs/prds/ to $ARTIFACTS_DIR/prds/ - Add conversationId to ApprovalOperationResult and RejectionOperationResult - Look up and pass through original conversation ID in CLI approve/reject commands - Add getConversationById mock to CLI workflow tests Fixes #1001, #1002, #1003
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR fixes approval gate response capture in the interactive-prd workflow and implements conversation ID preservation across approval and rejection operations. Changes include adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
Self Code ReviewSummaryAll three root causes correctly addressed. YAML fixes are clean, conversation threading is structurally sound. FindingsStrengths
Suggestions (non-blocking)
Security
Checklist
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.claude/PRPs/issues/issue-1001-1002-1003.md (1)
11-11: Normalize Markdown structure to remove lint noise.Line 11 jumps heading depth (
###) without a preceding##, and multiple fenced blocks are missing a language tag. This keeps triggering markdownlint warnings in PR artifacts.Also applies to: 70-70, 164-164, 171-171, 181-181, 191-191, 199-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/PRPs/issues/issue-1001-1002-1003.md at line 11, The markdown has inconsistent heading depth and missing fenced-code language tags causing lint warnings: change the "### Assessment" heading to the correct level (e.g., "## Assessment" or add the missing parent "##" above it) so heading hierarchy is sequential, and add explicit language identifiers (like ```md, ```text or the appropriate language) to all fenced code blocks referenced (the fenced blocks around the occurrences noted, including the blocks near the "Assessment" section and at the other reported locations). Ensure all headings follow increasing/decreasing levels without jumps and that every fenced block includes a language tag to satisfy markdownlint.packages/cli/src/commands/workflow.test.ts (1)
106-112: Add explicit assertions for conversation pass-through behavior.The new mock unblocks execution, but current tests don’t verify that approve/reject actually call
getConversationById(result.conversationId)and passplatform_conversation_idinto the resumed run path.As per coding guidelines, tests should validate command behavior in isolation with mocked dependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/workflow.test.ts` around lines 106 - 112, The test currently stubs getOrCreateConversation/getConversationById/updateConversation but doesn't assert that approve/reject call getConversationById(result.conversationId) or that the resumed run receives the conversation's platform_conversation_id; update the tests in workflow.test.ts to assert the mocked getConversationById was called with the returned result.conversationId (use the mock for getConversationById) and add an assertion that the resume/run-path (the function that continues execution after approval/rejection) was invoked with the platform_conversation_id value from the mocked conversation object (i.e., ensure the path that handles resuming receives 'platform_conversation_id' from getConversationById); keep the assertions isolated to the mocks getOrCreateConversation, getConversationById, and updateConversation so command behavior is validated without touching real dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/workflow.ts`:
- Around line 865-867: The DB lookup for the original conversation
(conversationDb.getConversationById(result.conversationId)) is happening outside
the resume error-handling path so a DB error can abort before the existing
resume/fallback logic runs; move the lookup into the same try/catch used for
resume (or add explicit error handling) inside the approve/reject command flow
so failures are caught and handled consistently—ensure you reference
originalConversation and platformConversationId after the lookup only when
present, and if the lookup fails either fall back intentionally (document the
behavior) or throw a clear error to fail fast.
---
Nitpick comments:
In @.claude/PRPs/issues/issue-1001-1002-1003.md:
- Line 11: The markdown has inconsistent heading depth and missing fenced-code
language tags causing lint warnings: change the "### Assessment" heading to the
correct level (e.g., "## Assessment" or add the missing parent "##" above it) so
heading hierarchy is sequential, and add explicit language identifiers (like
```md, ```text or the appropriate language) to all fenced code blocks referenced
(the fenced blocks around the occurrences noted, including the blocks near the
"Assessment" section and at the other reported locations). Ensure all headings
follow increasing/decreasing levels without jumps and that every fenced block
includes a language tag to satisfy markdownlint.
In `@packages/cli/src/commands/workflow.test.ts`:
- Around line 106-112: The test currently stubs
getOrCreateConversation/getConversationById/updateConversation but doesn't
assert that approve/reject call getConversationById(result.conversationId) or
that the resumed run receives the conversation's platform_conversation_id;
update the tests in workflow.test.ts to assert the mocked getConversationById
was called with the returned result.conversationId (use the mock for
getConversationById) and add an assertion that the resume/run-path (the function
that continues execution after approval/rejection) was invoked with the
platform_conversation_id value from the mocked conversation object (i.e., ensure
the path that handles resuming receives 'platform_conversation_id' from
getConversationById); keep the assertions isolated to the mocks
getOrCreateConversation, getConversationById, and updateConversation so command
behavior is validated without touching real dependencies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbd73ad5-e460-4581-a8a5-100f3b294d00
📒 Files selected for processing (5)
.archon/workflows/defaults/archon-interactive-prd.yaml.claude/PRPs/issues/issue-1001-1002-1003.mdpackages/cli/src/commands/workflow.test.tspackages/cli/src/commands/workflow.tspackages/core/src/operations/workflow-operations.ts
Collaborator
Author
PR Review SummaryCritical Issues (2 found)
Fix: Wrap both lookups in try-catch, log at let platformConversationId: string | undefined;
try {
const originalConversation = await conversationDb.getConversationById(result.conversationId);
platformConversationId = originalConversation?.platform_conversation_id ?? undefined;
} catch (error) {
const err = error as Error;
log.warn({ err, runId, conversationId: result.conversationId }, 'cli.conversation_lookup_failed');
}Important Issues (2 found)
Suggestions (3 found)
Strengths
Documentation Issues
CI Note
VerdictNEEDS FIXES — 2 critical (unguarded DB calls) + 1 important (untested happy path) Recommended Actions
|
instigator24
pushed a commit
to instigator24/Archon
that referenced
this pull request
Apr 9, 2026
…fix-output-format-approval fix: correct AI node options docs claiming loop node support (coleam00#987)
- Wrap getConversationById calls in try-catch in CLI approve/reject commands to prevent crashes when DB errors occur after approval is already recorded (falls back to new conversation ID) - Log when conversation lookup fails or returns null for observability - Add happy-path tests verifying platform conversation ID is passed through in both approve and reject commands - Add JSDoc comments clarifying conversationId is a DB UUID (not platform ID) in operation result interfaces and WorkflowRunOptions - Document approval: node type in CLAUDE.md DAG node types list
This was referenced Apr 9, 2026
leex279
pushed a commit
that referenced
this pull request
Apr 9, 2026
* Investigate issues #1001, #1002, #1003: interactive-prd workflow bugs * fix: interactive-prd workflow — capture responses, fix output path, reuse conversations (#1001, #1002, #1003) The archon-interactive-prd workflow was completely non-functional due to three bugs: approval gates didn't capture user responses (missing capture_response: true), the generate node wrote to .claude/ which the Claude SDK blocks, and CLI approve/reject created new conversations instead of reusing the original. - Add capture_response: true to all three approval gates in the YAML - Change output path from .claude/PRPs/prds/ to $ARTIFACTS_DIR/prds/ - Add conversationId to ApprovalOperationResult and RejectionOperationResult - Look up and pass through original conversation ID in CLI approve/reject commands - Add getConversationById mock to CLI workflow tests Fixes #1001, #1002, #1003 * fix: guard conversation lookups, add tests, document approval node type - Wrap getConversationById calls in try-catch in CLI approve/reject commands to prevent crashes when DB errors occur after approval is already recorded (falls back to new conversation ID) - Log when conversation lookup fails or returns null for observability - Add happy-path tests verifying platform conversation ID is passed through in both approve and reject commands - Add JSDoc comments clarifying conversationId is a DB UUID (not platform ID) in operation result interfaces and WorkflowRunOptions - Document approval: node type in CLAUDE.md DAG node types list
Merged
This was referenced Apr 11, 2026
Tyone88
pushed a commit
to Tyone88/Archon
that referenced
this pull request
Apr 16, 2026
…fix-output-format-approval fix: correct AI node options docs claiming loop node support (coleam00#987)
Tyone88
pushed a commit
to Tyone88/Archon
that referenced
this pull request
Apr 16, 2026
…eam00#1003) (coleam00#1005) * Investigate issues coleam00#1001, coleam00#1002, coleam00#1003: interactive-prd workflow bugs * fix: interactive-prd workflow — capture responses, fix output path, reuse conversations (coleam00#1001, coleam00#1002, coleam00#1003) The archon-interactive-prd workflow was completely non-functional due to three bugs: approval gates didn't capture user responses (missing capture_response: true), the generate node wrote to .claude/ which the Claude SDK blocks, and CLI approve/reject created new conversations instead of reusing the original. - Add capture_response: true to all three approval gates in the YAML - Change output path from .claude/PRPs/prds/ to $ARTIFACTS_DIR/prds/ - Add conversationId to ApprovalOperationResult and RejectionOperationResult - Look up and pass through original conversation ID in CLI approve/reject commands - Add getConversationById mock to CLI workflow tests Fixes coleam00#1001, coleam00#1002, coleam00#1003 * fix: guard conversation lookups, add tests, document approval node type - Wrap getConversationById calls in try-catch in CLI approve/reject commands to prevent crashes when DB errors occur after approval is already recorded (falls back to new conversation ID) - Log when conversation lookup fails or returns null for observability - Add happy-path tests verifying platform conversation ID is passed through in both approve and reject commands - Add JSDoc comments clarifying conversationId is a DB UUID (not platform ID) in operation result interfaces and WorkflowRunOptions - Document approval: node type in CLAUDE.md DAG node types list
This was referenced Apr 16, 2026
joaobmonteiro
pushed a commit
to joaobmonteiro/Archon
that referenced
this pull request
Apr 26, 2026
…fix-output-format-approval fix: correct AI node options docs claiming loop node support (coleam00#987)
joaobmonteiro
pushed a commit
to joaobmonteiro/Archon
that referenced
this pull request
Apr 26, 2026
…eam00#1003) (coleam00#1005) * Investigate issues coleam00#1001, coleam00#1002, coleam00#1003: interactive-prd workflow bugs * fix: interactive-prd workflow — capture responses, fix output path, reuse conversations (coleam00#1001, coleam00#1002, coleam00#1003) The archon-interactive-prd workflow was completely non-functional due to three bugs: approval gates didn't capture user responses (missing capture_response: true), the generate node wrote to .claude/ which the Claude SDK blocks, and CLI approve/reject created new conversations instead of reusing the original. - Add capture_response: true to all three approval gates in the YAML - Change output path from .claude/PRPs/prds/ to $ARTIFACTS_DIR/prds/ - Add conversationId to ApprovalOperationResult and RejectionOperationResult - Look up and pass through original conversation ID in CLI approve/reject commands - Add getConversationById mock to CLI workflow tests Fixes coleam00#1001, coleam00#1002, coleam00#1003 * fix: guard conversation lookups, add tests, document approval node type - Wrap getConversationById calls in try-catch in CLI approve/reject commands to prevent crashes when DB errors occur after approval is already recorded (falls back to new conversation ID) - Log when conversation lookup fails or returns null for observability - Add happy-path tests verifying platform conversation ID is passed through in both approve and reject commands - Add JSDoc comments clarifying conversationId is a DB UUID (not platform ID) in operation result interfaces and WorkflowRunOptions - Document approval: node type in CLAUDE.md DAG node types list
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
capture_response: trueto all three gates.claude/(blocked by Claude SDK) — use$ARTIFACTS_DIR/prds/insteadRoot Cause
Three independent bugs made
archon-interactive-prdcompletely non-functional:capture_responseexisted and never retrofitted.claude/PRPs/prds/which the Claude SDK blocksApprovalOperationResultlackedconversationId, so CLI always generated a new oneChanges
.archon/workflows/defaults/archon-interactive-prd.yamlcapture_response: trueto 3 gates; change output path to$ARTIFACTS_DIR/prds/packages/core/src/operations/workflow-operations.tsconversationIdtoApprovalOperationResultandRejectionOperationResult; return it from all pathspackages/cli/src/commands/workflow.tsconversationIdoption; look up original conversation in approve/reject commandspackages/cli/src/commands/workflow.test.tsgetConversationByIdto conversation DB mockTesting
Fixes #1001, #1002, #1003
Summary by CodeRabbit
Release Notes
New Features
approval:node type to pause workflow execution for human review, with optional response capture for downstream use.Improvements